conditional error reporting for vm move failure#13675
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #13675 +/- ##
=========================================
Coverage 16.26% 16.26%
- Complexity 13434 13435 +1
=========================================
Files 5667 5668 +1
Lines 500731 500734 +3
Branches 60803 60804 +1
=========================================
+ Hits 81455 81469 +14
+ Misses 410172 410161 -11
Partials 9104 9104
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
6cfbcc8 to
c03b25e
Compare
There was a problem hiding this comment.
Pull request overview
Adds a configurable way to return more informative API error messages when assignVirtualMachine (VM ownership change) fails, addressing the lack of actionable failure details reported in #13406.
Changes:
- Introduces a new global ConfigKey to control whether detailed assign/move failure reasons are exposed to API callers.
- Updates
AssignVMCmdto optionally returnInvalidParameterValueExceptionmessages to the caller when enabled. - Registers the new ConfigKey in
UserVmManagerImpl’s config key list.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | Registers the new configuration key for VM-assign failure detail exposure. |
| api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java | Conditionally returns a detailed failure reason from InvalidParameterValueException instead of only logging it. |
| api/src/main/java/com/cloud/vm/UserVmService.java | Defines the new global ConfigKey controlling detailed failure message exposure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| VmIpFetchThreadPoolMax, VmIpFetchTaskWorkers, AllowDeployVmIfGivenHostFails, EnableAdditionalVmConfig, DisplayVMOVFProperties, | ||
| KvmAdditionalConfigAllowList, XenServerAdditionalConfigAllowList, VmwareAdditionalConfigAllowList, DestroyRootVolumeOnVmDestruction, | ||
| EnforceStrictResourceLimitHostTagCheck, StrictHostTags, AllowUserForceStopVm}; | ||
| EnforceStrictResourceLimitHostTagCheck, StrictHostTags, AllowUserForceStopVm, AllowExposingVmAssignFailureDetails}; |
| public interface UserVmService { | ||
|
|
||
| ConfigKey<Boolean> AllowExposingVmAssignFailureDetails = new ConfigKey<>("Advanced", Boolean.class, | ||
| "vm.assign.failure.detailed.message.enabled", "true", |
| String msg = String.format("Failed to move Instance due to [%s].", getVmId()); | ||
| logger.error(msg, e); | ||
| if (e instanceof InvalidParameterValueException && UserVmService.AllowExposingVmAssignFailureDetails.value()) { | ||
| msg = e.getMessage(); | ||
| } |
|
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18663 |
Description
This PR...
Fixes: #13406
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?